Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#2273 When user selects and moves atom on Functional Group or Salt it doesn't replaces atom in the structure #2568

Merged
merged 4 commits into from
May 3, 2023
Merged

#2273 When user selects and moves atom on Functional Group or Salt it doesn't replaces atom in the structure #2568

merged 4 commits into from
May 3, 2023

Conversation

VasilevDO
Copy link
Collaborator

Closes #2273

@VasilevDO VasilevDO requested a review from Nitvex May 2, 2023 16:09
@VasilevDO VasilevDO linked an issue May 2, 2023 that may be closed by this pull request
When user selects and moves atom on Functional Group or Salt it doesn't replaces atom in the structure #2273
Closed
@KonstantinEpam23
Copy link
Collaborator

KonstantinEpam23 commented May 3, 2023 •
edited
Loading

@VasilevDO looks like this breaks the behaviour for adding a bond to a functional group.

Steps to reproduce: place a functional group on the canvas, select bond tool and click on the functional group.
Actual behaviour: Bond is not added to functional group.
Expected: Bond is added to attachment point atom of functional group.

[EDIT] Please ignore, I think I might have had some build problems, seems to work OK

KonstantinEpam23
KonstantinEpam23 reviewed May 3, 2023
View reviewed changes
packages/ketcher-react/src/script/editor/tool/helper/dropAndMerge.ts Outdated
}
const selectedSgroup = reStruct.sgroups.get(functionalGroupId)
const sGroupAtoms = SGroup.getAtoms(reStruct.molecule, selectedSgroup?.item)
const [firstAtom, ...atoms] = sGroupAtoms
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First atom is not guaranteed to be the attachment point, perhaps it is better to keep using sGroup?.getAttAtomId(struct) ?

@KonstantinEpam23 KonstantinEpam23 merged commit dff3ffc into release/2.10 May 3, 2023
@KonstantinEpam23 KonstantinEpam23 deleted the #2273-select-atom-drag-and-drop-to-functional-group branch May 3, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnastasiiaPlyako AnastasiiaPlyako AnastasiiaPlyako approved these changes

@KonstantinEpam23 KonstantinEpam23 KonstantinEpam23 approved these changes

@Nitvex Nitvex Awaiting requested review from Nitvex

Assignees

@VasilevDO VasilevDO

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When user selects and moves atom on Functional Group or Salt it doesn't replaces atom in the structure
3 participants
@VasilevDO @KonstantinEpam23 @AnastasiiaPlyako